Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move segments of library to separate crates #164

Merged
merged 31 commits into from
Apr 13, 2016
Merged

Move segments of library to separate crates #164

merged 31 commits into from
Apr 13, 2016

Conversation

hauleth
Copy link
Member

@hauleth hauleth commented Feb 15, 2016

Issue #102

  • traits
  • bigint
  • integer
  • complex
  • iter
  • rational

@hauleth hauleth added this to the v0.2.0 milestone Feb 15, 2016
@hauleth hauleth self-assigned this Feb 15, 2016
@cuviper
Copy link
Member

cuviper commented Feb 16, 2016

It's not necessarily a breaking change, if all of the same paths are still available through num::foo.

The travis scripts will need to be updated to make sure all tests are run across all subcrates. Some of that is already extracted into separate shell scripts, so if it starts getting long we could pull the entire script: section into a real master script.

@hauleth
Copy link
Member Author

hauleth commented Feb 16, 2016

@cuviper I have changed some traits (i.e. content of Num trait was moved to the FromStrRadix) so it can break some apps.

@cuviper
Copy link
Member

cuviper commented Feb 16, 2016 via email

@hauleth
Copy link
Member Author

hauleth commented Feb 16, 2016

No, it is not necessary. I will extract this.

Łukasz Jan Niemier

Dnia 16 lut 2016 o godz. 19:18 Josh Stone [email protected] napisał(a):

OK, I hadn't looked that closely since it's a WIP. FromStrRadix sounds
like a reasonable but orthogonal change. Is it really necessary for the
split? Otherwise I'd do that separately.

Reply to this email directly or view it on GitHub.

@homu
Copy link
Contributor

homu commented Feb 16, 2016

☔ The latest upstream changes (presumably ebed675) made this pull request unmergeable. Please resolve the merge conflicts.

@kamalmarhubi
Copy link

I don't have a preference between this and my #174, except that it should happen sooner than later. Getting the traits into their own crate is already done, either here or there. If you get that merged separately, you can release the num-traits crate sooner. :-)

@hauleth hauleth force-pushed the split-into-crates branch 4 times, most recently from 7cf801b to 3b526cd Compare March 4, 2016 11:52
@homu
Copy link
Contributor

homu commented Mar 5, 2016

☔ The latest upstream changes (presumably #176) made this pull request unmergeable. Please resolve the merge conflicts.

@kamalmarhubi
Copy link

@hauleth can we get the traits change landed and released sooner? My thinking is that this won't be a breaking change for any users so there's no need to delay landing any of it.

@kamalmarhubi
Copy link

@hauleth

@cuviper I have changed some traits (i.e. content of Num trait was moved to the FromStrRadix) so it can break some apps.

Well, aside from this being breaking. I think it's orthogonal to the crate refactor and should be done either before or after the multi-crate change.

@hauleth
Copy link
Member Author

hauleth commented Mar 10, 2016

I should finish this later today, so You can expect release today (UTC+1 timezone).

@kamalmarhubi
Copy link

@hauleth ! yay!

@cuviper
Copy link
Member

cuviper commented Mar 29, 2016

Aside, here's a neat way to test subpackages, pulling each lib.rs as a top-level [[test]].
https://users.rust-lang.org/t/cargo-test-internal-packages/5187/3

@hauleth
Copy link
Member Author

hauleth commented Mar 29, 2016

@cuviper this hack is nice, but not as it would satisfy me. It requires that root crate will have the same dependencies as all sub-crates. This is kind of no-go for us as I've splitted dependencies. What is more I want to make num-traits #![no_std] which would make whole testing suite fail. So currently our only solution is to use make.

@hauleth
Copy link
Member Author

hauleth commented Mar 29, 2016

@cuviper are we ready with this?

@cuviper
Copy link
Member

cuviper commented Mar 29, 2016

We need better subcrate descriptions, but otherwise I think it's good.

@bluss
Copy link
Contributor

bluss commented Apr 6, 2016

Will you bump the versions when you do this?

I have lots of ideas for Float that I'd like to explore if we have a new version. Ideally they would be in place in the first release of a new major version.

@cuviper
Copy link
Member

cuviper commented Apr 6, 2016

No bump yet, I think semver should be OK through this one.
(Please speak up if you think there's a break!)

@hauleth
Copy link
Member Author

hauleth commented Apr 6, 2016

I will finish it tomorrow. What is left is descriptions and publishing all packages.

I think that we should tag each of the subcrates independently via <subcrate name without num_* prefix>/<version>.

@cuviper
Copy link
Member

cuviper commented Apr 9, 2016

@hauleth Have you worked on those descriptions? If not, I can get to it tonight.

@hauleth
Copy link
Member Author

hauleth commented Apr 10, 2016

Here you are.

@cuviper
Copy link
Member

cuviper commented Apr 10, 2016

I'd prefer to keep a fuller description on the main num crate, but I'll tweak that myself.

I also just noticed that features aren't trickling down to subcrates correctly. For example, enabling serde at the top doesn't enable it in bigint. (which is broken anyway, missing extern crate serde;)

@hauleth
Copy link
Member Author

hauleth commented Apr 11, 2016

@cuviper I know no solution to push flags to sub crates. Maybe there should be an RFC for that.

@bluss
Copy link
Contributor

bluss commented Apr 11, 2016

I don't think there is a solution if the feature name is the same name as a dependency in the same crate. Otherwise you can forward like this:

[features]
serde = ["num-bigint/serde"]   # Requires same feature from dependency num-bigint

It is not conditionally forwarding.

This issue is semi related, isn't it? rust-lang/cargo#1286

@cuviper
Copy link
Member

cuviper commented Apr 11, 2016

Ah, but the top-level num crate itself doesn't actually need any of those external dependencies (rand, rustc-serialize, serde). So perhaps they can be left as abstract dependencies there, and then it should forward correctly, right?

Then for instance rational optionally uses bigint, but since the actual crate is named num-bigint, I think expanding num-rational/bigint may work. We may need default-features = false in the top Cargo.toml's rational section to allow fine control.

@hauleth
Copy link
Member Author

hauleth commented Apr 11, 2016

Should be fixed now.

@cuviper
Copy link
Member

cuviper commented Apr 13, 2016

Let's merge the beast. I can try to publish tonight, if you don't get to it first.
May want to get #182 in there too - it's a simple regression fix.

@homu r+ 58b5fe5

@homu homu merged commit 58b5fe5 into master Apr 13, 2016
@homu
Copy link
Contributor

homu commented Apr 13, 2016

⚡ Test exempted - status

homu added a commit that referenced this pull request Apr 13, 2016
Move segments of library to separate crates

Issue #102

- [x] traits
- [x] bigint
- [x] integer
- [x] complex
- [x] iter
- [x] rational
@hauleth hauleth deleted the split-into-crates branch April 13, 2016 21:36
@cuviper
Copy link
Member

cuviper commented Apr 14, 2016

This is all published now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants